Fix QuickNAT indexed skip connections by yinkev · Pull Request #9021 · Project-MONAI/MONAI

yinkev · 2026-07-25T19:45:56Z

Description

QuickNAT's indexed skip connection now invokes its indexed submodule with the required (input, indices) signature, forwards the exact incoming index object through the nested indexed path, preserves it for the decoder, supports cat, add, and mul fusion modes, and rejects unsupported modes explicitly.

The prior implementation delegated to ordinary SkipConnection.forward(), which calls its submodule with only the input tensor. QuickNAT's nested SequentialWithIdx requires both arguments, so se_block=None failed with SequentialWithIdx.forward() missing ... 'indices'. Existing tests were skipped whenever the optional squeeze-and-excitation package was unavailable, leaving the core path uncovered.

The public se_block annotation now also reflects the already-supported None configuration.

Types of changes

Non-breaking change (fix or new feature that would not break existing functionality).
Breaking change (fix or new feature that would cause existing functionality to change).
New tests added to cover the changes.
Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
In-line docstrings updated.
Documentation updated, tested make html command in the docs/ folder.

Validation

core regression fails on upstream/dev with the missing indices argument;
review regression proves that the exact incoming index object reaches the nested indexed submodule;
focused core and fusion-mode tests: 5 passed;
QuickNAT unittest module: 14 tests, 5 passed and 9 expected optional-dependency skips;
Black, isort, Ruff, DCO, and git diff --check pass.

Signed-off-by: kyinhub <kevinpyin@gmail.com>

coderabbitai · 2026-07-25T19:46:18Z

📝 Walkthrough

Walkthrough

QuickNAT’s indexed skip connection now computes its submodule output and supports cat, add, and mul fusion modes while preserving indices and rejecting unsupported modes. The se_block parameter accepts None in its annotation and documentation. Tests cover fusion behavior, error handling, index preservation, and forward execution without squeeze-and-excitation.

Estimated code review effort: 2 (Simple) | ~15 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes address `#7475` by forwarding indices correctly and adding independent core tests for QuickNAT.
Out of Scope Changes check	✅ Passed	The edits stay within the QuickNAT bugfix scope and the added tests support the same objective.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title is concise and accurately summarizes the main change to QuickNAT indexed skip connections.
Description check	✅ Passed	The description matches the template: issue link, summary, change types, and validation notes are provided.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (1)

tests/networks/nets/test_quicknat.py (1)
28-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Align the new test code with Python style rules.

Rename input to input_tensor (Ruff A002/A001), and expand _DoubleWithIdx.forward with Google-style Args and Returns sections.

Also applies to: 52-56
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/networks/nets/test_quicknat.py` around lines 28 - 33, Update
_DoubleWithIdx.forward to rename the input parameter from input to input_tensor
and update its usage, then expand the docstring with Google-style Args and
Returns sections documenting input_tensor, indices, and the returned tuple.
Sources: Path instructions, Linters/SAST tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@monai/networks/nets/quicknat.py`:
- Line 58: Update the QuickNat forward path in monai/networks/nets/quicknat.py
at lines 58-58 to pass the existing indices argument to self.submodule instead
of None. Add or update the test in tests/networks/nets/test_quicknat.py at lines
55-63 so _DoubleWithIdx verifies it receives the identical indices object.

In `@tests/networks/nets/test_quicknat.py`:
- Around line 55-63: Update the test around SkipConnectionWithIdx to verify that
the wrapped _DoubleWithIdx submodule receives the original indices argument, not
None. Capture the submodule’s received indices or make it reject None, then
assert object identity against indices in addition to validating the output and
returned indices.

---

Nitpick comments:
In `@tests/networks/nets/test_quicknat.py`:
- Around line 28-33: Update _DoubleWithIdx.forward to rename the input parameter
from input to input_tensor and update its usage, then expand the docstring with
Google-style Args and Returns sections documenting input_tensor, indices, and
the returned tuple.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 92b60d12-343c-464e-aabc-472d8cd13578

📥 Commits

Reviewing files that changed from the base of the PR and between 3ee058b and 798a88d.

📒 Files selected for processing (2)

monai/networks/nets/quicknat.py
tests/networks/nets/test_quicknat.py

Signed-off-by: kyinhub <kevinpyin@gmail.com>

fix(networks): support QuickNAT without optional SE blocks

798a88d

Signed-off-by: kyinhub <kevinpyin@gmail.com>

yinkev requested review from KumoLiu, Nic-Ma and ericspod as code owners July 25, 2026 19:45

yinkev mentioned this pull request Jul 25, 2026

Fix QuickNAT indexed skip connections #9015

Closed

7 tasks

coderabbitai Bot reviewed Jul 25, 2026

View reviewed changes

Comment thread monai/networks/nets/quicknat.py Outdated

Comment thread tests/networks/nets/test_quicknat.py Outdated

yinkev marked this pull request as draft July 25, 2026 20:59

fix(networks): forward QuickNAT skip indices

fa90c9f

Signed-off-by: kyinhub <kevinpyin@gmail.com>

yinkev marked this pull request as ready for review July 26, 2026 06:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix QuickNAT indexed skip connections#9021

Fix QuickNAT indexed skip connections#9021
yinkev wants to merge 2 commits into
Project-MONAI:devfrom
yinkev:7475-quicknat-indices

yinkev commented Jul 25, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 25, 2026 •

edited

Loading

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

yinkev commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Validation

Uh oh!

coderabbitai Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

yinkev commented Jul 25, 2026 •

edited

Loading

coderabbitai Bot commented Jul 25, 2026 •

edited

Loading